feat(preprod): Add snapshots subcommand by lcian · Pull Request #3110 · getsentry/sentry-cli

lcian · 2026-01-28T09:49:51Z

Updated version of #3049 to discuss and iterate on things.

Notable changes:

Removed shard_index parameter from the command. I'm not sure what the purpose of that was originally.
This uses the new many (batch) API from objectstore_client. All uploads are executed as batch requests, reducing network overhead. Unfortunately, with they way things are implemented now, we will still have to buffer all files in memory before sending the request, as we need to hash their contents to determine the filename. If we could just use the filename as the key in objectstore, it would be much better because that way we could stream the files over.

Note that auth enforcement still needs to be enabled for objectstore, so that's currently blocking this to be used for anything but internal testing.

Ref FS-233

lcian · 2026-01-28T10:01:42Z

src/api/mod.rs

+#[derive(Deserialize, Debug)]
+pub struct OrganizationDetails {
+    pub id: String,
+    pub links: OrganizationLinks,


This endpoint has a bunch more information but what we really care about is the id and links.
The region_url contains the regional API url, e.g. us.sentry.io. This way we can hit that endpoint directly instead of going through control silo (sentry.io).

If I understand correctly I think this information might already be somehow embedded in org tokens, but not in personal tokens. Or maybe I'm confusing this with something else.

If I understand correctly I think this information might already be somehow embedded in org tokens, but not in personal tokens

This is 100% correct

Ok then I should see how to resolve the id from the token instead of using the API call when we have an org token.

To clarify, only the region URL is embedded. Org ID is not

Let's discuss about the need to resolve the org and project IDs in the first place. Almost all other endpoints are capable of accepting both slugs and IDs interchangeably in the backend (Sentry side). To me it seems like a pretty serious limitation if objectstore cannot accept either.

That said, what you mentioned about having arbitrary keys in your other comment makes sense, and I guess it makes it harder to accept both IDs and slugs. Let's see if we can come up with a solution here, though

lcian · 2026-01-28T10:06:26Z

src/utils/api/mod.rs

+use anyhow::{Context, Result};
+
+/// Given an org and project slugs or IDs, returns the IDs of both.
+pub fn get_org_project_id(api: impl AsRef<Api>, org: &str, project: &str) -> Result<(u64, u64)> {


As far as I know, for all commands a user can provide --project and --org as slugs or IDs.
These utils are needed to get the corresponding IDs, so that objects from the same org/proj all have the same paths in objectstore regardless if the user passed in slugs or IDs.

It might be weird to take in Api and it might be possible that these functions should live somewhere else, IDK.
They certainly don't belong to the Api struct as its methods seem to all map 1to1 to Sentry API calls.

Can the backend interpret the slugs in to ids instead of the frontend?

Can the backend interpret the slugs in to ids instead of the frontend?

Indeed, I agree that this would be preferable. The current code adds two additional API calls that could be avoided if the backend resolves the slugs/IDs appropriately.

Not sure what backend and frontend here refers to.
How objectstore works is that the scope (org and project in this case) is an arbitrary sequence of key-value pairs. We recommend using org and project ID, or at least org ID, but that's not mandatory.
Therefore it's not a responsibility of objectstore or its endpoint in the monolith to normalize org and project, we simply take what the user provides. It's responsibility of the user to normalize.

lcian · 2026-01-28T10:11:13Z

Cargo.toml

 lazy_static = "1.4.0"
 libc = "0.2.139"
 log = { version = "0.4.17", features = ["std"] }
+objectstore-client = { git =  "https://github.com/getsentry/objectstore.git", branch = "lcian/feat/rust-batch-client" }


This indirectly adds a dependency to reqwest and we need to double check what the implications of that are, especially in regard to rustls vs native-tls which could be problematic.

It also adds a bunch of deps but I think most of them are inevitable.

Let's discuss the TLS stuff next week, because I am uncertain what problems you're concerned about.

linear · 2026-01-28T10:15:48Z

FS-233 Add new command to Sentry CLI to upload to Objectstore

lcian · 2026-02-03T00:20:58Z

src/commands/build/snapshots.rs

+    let client = Client::new(url)?;
+
+    let (org, project) = get_org_project_id(Api::current(), org, project)?;
+    let session = Usecase::new("preprod")


We should set an expiration policy here.
I think based on our discussions TTL should be ideal for this usecase?

As we discussed we want different TTL based on customer/plan. We could introduce a new endpoint in sentry that tells you this info and set the TTL here based on that.

Yep, for now I think we're good with nothing set just for testing, but I'll track this here for my team to follow up on in the near future: https://linear.app/getsentry/issue/EME-839/endpoint-for-determining-ttl-for-an-org

cursor

Cursor Bugbot has reviewed your changes and found 3 potential issues.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

cursor · 2026-02-05T18:38:15Z

src/commands/build/snapshots.rs

+            // For other files, use {org}/{project}/{hash}
+            let hash = compute_sha256_hash(&contents);
+            format!("{org}/{project}/{hash}")
+        };


JSON file keys missing project ID causes cross-project collisions

High Severity

JSON file keys use the format {org}/{snapshot_id}/{filename} which excludes the project ID, while non-JSON files use {org}/{project}/{hash} which includes it. Since the command requires --project and is documented as uploading "build snapshots to a project", omitting the project ID from JSON file keys causes different projects within the same organization to overwrite each other's files when using the same snapshot_id and filename.

I'll handle this and other business logic related to the emerge snapshot features

cursor · 2026-02-05T18:38:15Z

src/commands/build/snapshots.rs

+                    files.push(entry_path.to_path_buf());
+                }
+            }
+        }


Hidden directories not skipped, exposing sensitive files

Medium Severity

The WalkDir traversal descends into hidden directories (e.g., .git, .svn, .env.d/) but is_hidden_file only checks if the file's own name starts with .. Files inside hidden directories with non-hidden names (like .git/config, .git/objects/*) are uploaded despite the intent to skip hidden content. This could inadvertently upload sensitive data or significantly bloat uploads with unwanted files.

Additional Locations (1)

src/commands/build/snapshots.rs#L120-L126

cursor · 2026-02-05T18:38:15Z

src/commands/build/snapshots.rs

+                .file_name()
+                .and_then(|name| name.to_str())
+                .unwrap_or("unknown.json");
+            format!("{org}/{snapshot_id}/{filename}")


JSON files in subdirectories with same filename overwrite each other

Medium Severity

For JSON files, the key is generated using only file_name() which extracts just the base filename, discarding any subdirectory path. If a user uploads a directory containing subdir1/data.json and subdir2/data.json, both files produce the identical key {org}/{snapshot_id}/data.json, causing the second file to silently overwrite the first and resulting in data loss.

szokeasaurusrex · 2026-02-06T15:04:56Z

Hey @lcian, are you ready for me to review this, or are you still planning to check the feedback from Bugbot and/or iterate further?

rbro112 · 2026-02-10T05:47:04Z

feat(preprod): Add snapshots subcommand #3110 👈 (View in Graphite)
master

This stack of pull requests is managed by Graphite. Learn more about stacking.

Adds the initial snapshots POST API. This api does a few things and is intended to be invoked by the CLI: - Creates `PreprodArtifact`, `PreprodSnapshotMetrics` and `CommitComparison` DB models - Creates image metadata based on what's uploaded from CLI - Stores metadata in objectstore Notably images are uploaded directly to objectstore from CLI Tested E2E locally with objectstore and WIP CLI branch (getsentry/sentry-cli#3110) Resolves EME-773

szokeasaurusrex

I left some comments. Most of them are minor and optional, but I would call your attention to the comment about memory usage and the potential for leaking tokens.

Let's discuss further next week

szokeasaurusrex · 2026-02-13T09:47:42Z

src/api/mod.rs

+#[derive(Deserialize, Debug)]
+pub struct OrganizationLinks {
+    #[serde(rename = "regionUrl")]
+    pub region_url: String,
+}


nit: you can use rename_all to avoid duplicating the field name

Suggested change

#[derive(Deserialize, Debug)]

pub struct OrganizationLinks {

#[serde(rename = "regionUrl")]

pub region_url: String,

}

#[derive(Deserialize, Debug)]

#[serde(rename_all = "camelCase")]

pub struct OrganizationLinks {

pub region_url: String,

}

szokeasaurusrex · 2026-02-13T09:49:22Z

src/api/mod.rs

+#[derive(Deserialize, Debug)]
+pub struct OrganizationDetails {
+    pub id: String,
+    pub links: OrganizationLinks,


If I understand correctly I think this information might already be somehow embedded in org tokens, but not in personal tokens

This is 100% correct

szokeasaurusrex · 2026-02-13T09:50:51Z

src/commands/build/snapshots.rs

+#[derive(Deserialize)]
+struct CreateSnapshotResponse {
+    #[serde(rename = "artifactId")]
+    artifact_id: String,
+    #[serde(rename = "imageCount")]
+    image_count: u64,
+}


l:

Suggested change

#[derive(Deserialize)]

struct CreateSnapshotResponse {

#[serde(rename = "artifactId")]

artifact_id: String,

#[serde(rename = "imageCount")]

image_count: u64,

}

#[derive(Deserialize)]

#[serde(rename_all = "camelCase")]

struct CreateSnapshotResponse {

artifact_id: String,

image_count: u64,

}

szokeasaurusrex · 2026-02-13T09:53:32Z

src/commands/build/snapshots.rs

+    // Build manifest from discovered images
+    let mut manifest = json!({
+        "app_id": app_id,
+        "images": {},
+    });


m: Why is this being custom-constructed as a JSON object?

I think it might make more sense to make a dedicated struct type for this manifest. Is there any reason why this was not done?

szokeasaurusrex · 2026-02-13T09:54:10Z

src/commands/build/snapshots.rs

+    let images_obj = manifest["images"]
+        .as_object_mut()
+        .expect("images object was just created");
+    for image in &images {
+        images_obj.insert(
+            image.hash.clone(),
+            json!({
+                "file_name": image.relative_path,
+                "width": image.width,
+                "height": image.height,
+            }),
+        );
+    }


l: Following from my comment above, if we make a struct Manifest, this logic could be an implementation on the struct.

szokeasaurusrex · 2026-02-13T13:20:00Z

Cargo.toml

 lazy_static = "1.4.0"
 libc = "0.2.139"
 log = { version = "0.4.17", features = ["std"] }
+objectstore-client = { git =  "https://github.com/getsentry/objectstore.git", branch = "lcian/feat/rust-batch-client" }


Let's discuss the TLS stuff next week, because I am uncertain what problems you're concerned about.

szokeasaurusrex · 2026-02-13T13:20:10Z

Cargo.toml

 lazy_static = "1.4.0"
 libc = "0.2.139"
 log = { version = "0.4.17", features = ["std"] }
+objectstore-client = { git =  "https://github.com/getsentry/objectstore.git", branch = "lcian/feat/rust-batch-client" }


h: Before we merge these changes, we should ensure we are depending on a version that's been published to crates.io, not a Git branch.

szokeasaurusrex · 2026-02-13T13:22:25Z

src/commands/build/snapshots.rs

+    };
+
+    let url = get_objectstore_url(Api::current(), org)?;
+    let client = ClientBuilder::new(url.clone())


l: clone() appears to be unnecessary

Suggested change

let client = ClientBuilder::new(url.clone())

let client = ClientBuilder::new(url)

szokeasaurusrex · 2026-02-13T13:23:46Z

src/utils/api/mod.rs

l: I would move this to src/utils/api.rs for now

szokeasaurusrex · 2026-02-13T13:24:06Z

src/utils/objectstore/mod.rs

l: Would move to src/utils/objectstore.rs for now.

noahsmartin and others added 8 commits December 17, 2025 20:02

feat(preprod): Add snapshots subcommand

95876c4

update deps

e45f588

no multiple projects, get rid of shard index

f3cfadb

add organization details API util

4794046

introduce necessary utils

c6e67e9

use batch API

1cd38b7

improve

15a89af

improve

1bc440d

This comment was marked as outdated.

Sign in to view

cargo clippy --fix

3417b53

lcian commented Jan 28, 2026

View reviewed changes

improve

3f206dd

lcian commented Jan 28, 2026

View reviewed changes

fmt all

e902bfc

fix test

e5bd2d7

lcian commented Feb 3, 2026

View reviewed changes

lcian added 4 commits February 5, 2026 08:47

add auth, fix url

081e550

cargo clippy --fix

d13ddec

changelog

db855ee

Merge branch 'master' into lcian/feat/snapshots

b70e7b1

lcian marked this pull request as ready for review February 5, 2026 18:32

lcian requested review from a team and szokeasaurusrex as code owners February 5, 2026 18:32

cursor bot reviewed Feb 5, 2026

View reviewed changes

Updates to get e2e run working

0d59c90

rbro112 mentioned this pull request Feb 10, 2026

feat(preprod): Snapshots upload API getsentry/sentry#107825

Merged

szokeasaurusrex reviewed Feb 13, 2026

View reviewed changes

	let client = ClientBuilder::new(url.clone())
	let client = ClientBuilder::new(url)

Uh oh!

Conversation

lcian commented Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

lcian Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

linear bot commented Jan 28, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor bot Feb 5, 2026

Choose a reason for hiding this comment

JSON file keys missing project ID causes cross-project collisions

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cursor bot Feb 5, 2026

Choose a reason for hiding this comment

Hidden directories not skipped, exposing sensitive files

Uh oh!

cursor bot Feb 5, 2026

Choose a reason for hiding this comment

JSON files in subdirectories with same filename overwrite each other

Uh oh!

szokeasaurusrex commented Feb 6, 2026

Uh oh!

rbro112 commented Feb 10, 2026

Uh oh!

szokeasaurusrex left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

lcian commented Jan 28, 2026 •

edited

Loading

lcian Jan 28, 2026 •

edited

Loading

szokeasaurusrex left a comment •

edited

Loading